-
Notifications
You must be signed in to change notification settings - Fork 26
remove grid-scale thermo state #4231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
c98b81f to
e95e753
Compare
4dfb4ea to
934de93
Compare
4fd4cd6 to
953e904
Compare
|
working atmos longrun: https://buildkite.com/clima/climaatmos-gpulongruns/builds/712 |
141522a to
e4892be
Compare
tapios
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Amazing work, and huge lift. Thank you!
|
|
||
| # Saturation state at current thermodynamic state (via θ, p, q_tot) | ||
| ts_current = TD.PhaseEquil_pθq(thermo_params, p_c, θli, q_tot) | ||
| q_sat = TD.q_vap_saturation(thermo_params, ts_current) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be more straightforward to pass in temperature rather than pressure, and then compute q_sat and other quantities we need from that. Wouldn't that avoid use of PhaseEquil... here? (Same for the other cloud_fraction function above.)
Perhaps your intention was to do that, or something like it, in a separate PR, and that's fine too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I want to remove the PhaseEquil in the next PR, partially because I didn't know how to deal with this field of named tuple. But now I think I know:)
| end | ||
|
|
||
| function saturation_distance(q_tot, q_sat, ᶜts, θli, thermo_params, Δθli_fd) | ||
| function saturation_distance(q_tot, q_sat, p_c, θli, thermo_params, Δθli_fd) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above: I think this gets easier to change, and to reduce reliance on equilibrium constructors, by passing in temperature rather than pressure.
src/cache/precomputed_quantities.jl
Outdated
| @. ᶜts = ts_gs(thermo_args..., Y.c, ᶜK, ᶜΦ, Y.c.ρ) | ||
| @. ᶜp = TD.air_pressure(thermo_params, ᶜts) | ||
| # Compute thermodynamic state variables using individual getter functions. | ||
| # Note: For EquilMoistModel, this calls saturation_adjustment 3 times per grid point |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above: why not combining getters so that we call SA only once?
src/parameterized_tendencies/gravity_wave_drag/orographic_gravity_wave.jl
Show resolved
Hide resolved
fdd4b9a to
86cce9c
Compare
86cce9c to
fd8bef0
Compare
Purpose
Removes grid-scale thermo state and uses new Thermodynamics API.
Removes ts and adds ᶜT, ᶜq_tot_safe, ᶜq_liq_rai, ᶜq_ice_sno to the cache. Also adds h_tot back to the cache because using
@lazyresults in gpu error. The difference between ᶜq_tot_safe and ᶜq_tot (a lazy broadcasting) is ᶜq_tot_safe is clipped to 0. The difference between ᶜq_liq_rai, ᶜq_liq and ᶜq_rai is ᶜq_liq_rai = ᶜq_liq + ᶜq_rai and clipped to 0. Same for ᶜq_ice_sno. I am open to different names. ᶜT, ᶜq_tot_safe, ᶜq_liq_rai, ᶜq_ice_sno essentially create the previous thermo state. Clipping the humidities to 0 recovers the previous behavior of thermo state.I will remove subgrid thermo state and remaining TD.PhaseEquil in separate PRs.
To-do
Content